-
Notifications
You must be signed in to change notification settings - Fork 447
feat: Instantiation payload support for INetworkPrefabInstanceHandler #3430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop-2.0.0
Are you sure you want to change the base?
Conversation
…efabInstanceHandler.Instantiate() documentation
I just posted a video demonstrating how the system works:
In the video, I spawn The
2025-04-27.21-45-44.mp4By implementing a custom Here is the core implementation: public class TestHandlerDeterministicLink : INetworkPrefabInstanceHandler, INetworkInstantiationPayloadSynchronizer
{
public Dictionary<int, DeterministicIDHolder> deterministicSpawns = new Dictionary<int, DeterministicIDHolder>();
public int customSpawnID = 0;
void INetworkInstantiationPayloadSynchronizer.OnSynchronize<T>(ref BufferSerializer<T> serializer) => serializer.SerializeValue(ref customSpawnID);
public NetworkObject Instantiate(ulong clientId, Vector3 position, Quaternion rotation)
{
var obj = deterministicSpawns[customSpawnID];
TMP_Text text = obj.GetComponent<TMP_Text>();
text.SetText(text.text + "*");
return obj.GetComponent<NetworkObject>();
}
public void Destroy(NetworkObject networkObject) => GameObject.Destroy(networkObject.gameObject);
int nextDeterministicId = 0;
public void InstantiateLocally(GameObject linkablePrefab)
{
var spawned = GameObject.Instantiate(linkablePrefab);
spawned.transform.position = UnityEngine.Random.insideUnitCircle * 0.01f;
var text = spawned.GetComponent<TMP_Text>();
text.SetText(nextDeterministicId.ToString() + "&" + text.text);
var deterministicIdHolder = spawned.GetComponent<DeterministicIDHolder>();
deterministicSpawns[nextDeterministicId] = deterministicIdHolder;
deterministicIdHolder.SetID(nextDeterministicId);
nextDeterministicId++;
}
} Warning While this system enables advanced workflows, |
This would actually save us a lot of trouble. Right now when after we spawn stuff we have to run like two or three RPCs just to finish setting up objects properly. I wish you luck on get it merged on Unity 6.1, it would be super useful for our current project. |
Sure! I'm just waiting for reviewers to be assigned to this PR. |
I just got more feedback on the Issue #3421 I'm thinking about a way to have the prefab handler "write" the payload right before the spawn happens. In this approach, I would try to move most of the logic into CreateObjectMessage, removing it from the object data. This would avoid all the newly added generics and any potential object boxing. I'm converting this PR into a draft to keep modifying the implementation and will get back to comment once it's ready for feedback again. |
[Sorry bad writting i might edit this text later] I did requested changes by @EmandM into the PR, currently im reusing the same buffer serializer from the object serialization. Also i could make the new interface to not have the OnSynchronize, and having instead Serialize and Deserialize methods, but that would make the usage not as comfortable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fantastic next step! The video was super helpful to understand what the intention was. Is there any chance you have a more minimalistic example of this feature that doesn't require linking the items together later. The added complexity of having separate objects that are linked implies that this feature is doing the linking. My understanding is simply that this feature enables changing the object that is instantiated as part of the instantiation flow.
A few notes on the code:
Out of interest, is there a reason you chose to implement the custom data passing only on scene objects? We'd prefer a solution that works everywhere where the prefab handler can work. Again, the symmetry in the approach is important. If you can do something with prefab handlers in one area, that feature should work in all areas.
It would also be fantastic if you can add a test showing how this feature is used.
/// Interface for synchronizing custom instantiation payloads during NetworkObject spawning. | ||
/// Used alongside <see cref="INetworkPrefabInstanceHandler"/> to extend instantiation behavior. | ||
/// </summary> | ||
public interface INetworkInstantiationPayloadSynchronizer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name could be more descriptive. How about something like INetworkPrefabInstantiationHandler
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this interface doesn't handle instantiation itself, that’s entirely the role of INetworkPrefabInstanceHandler
.
The purpose of INetworkInstantiationPayloadSynchronizer
is strictly to serialize and deserialize the instantiation payload before Instantiate() is called. That’s why I went with a name that emphasizes its function in data synchronization, rather than suggesting it’s involved in the instantiation logic directly.
If we go with the OnPreInstantiate
or OnBeforeInstantiation
naming you suggested, perhaps something like INetworkPrefabPreInstantiationHandler
or INetworkPrefabBeforeInstantiationHandler
would better reflect the purpose. I’m happy to update the name as long as it clearly communicates what the interface does.
I used the term payload since it directly refers to the custom data being passed along with the spawn message, which is exactly what this interface handles
I'll explore a few alternative naming options in tomorrow commit to see if any feel like a better fit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Along with this note, I was also thinking it might be nice if this new interface extends from the base interface.
So rather than needing
class MyHandler : INetworkPrefabInstanceHandler, INetworkInstantiationPayloadSynchronizer
It could instead be used as
class MyHandler : INetworkPrefabPayloadHandler
or something of that type. Keeps it clearer for developers to implement and simpler to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty good idea, i like how it looks simpler now, something like INetworkPefabInstanceHandlerWithData
would be similar to the old namings of the job system, like IJobComponent IJobWithECB and similar.
INetworkPrefabPayloadHandler
looks good to me but I feel INetworkPefabInstanceHandlerWithData
its even more descriptive for developers who will use it.
What do you think, could we name it INetworkPefabInstanceHandlerWithData
?
Im sure we can find a better naming later for the second interface it wraps.
/// allowing you to cache or prepare information needed during instantiation. | ||
/// </summary> | ||
/// <param name="serializer">The buffer serializer used to read or write custom instantiation data.</param> | ||
void OnSynchronize<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid confusion with NetworkBehaviours, could we rename this to something more descriptive like OnInstantiation()
or OnBeforeInstantiation()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me! As long as the name clearly reflects that this method is solely responsible for synchronizing data prior to instantiation, I’m happy to update it.
OnBeforeInstantiation works well for that, as long as we keep in mind that it is actually serializing and deserializing data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might rename the interface to something like:
INetworkPrefabPreInstantianceDataSerializer
INetworkPrefabPreInstantianceSynchronizer
INetworkPrefabPreInstantiantiateHandler
And the method could be:
OnPreInstanceSerialization
, SynchronizePreInstance
, PreInstanceSynchronization
, HandlePreInstantiateData
This way, the naming makes it clearer that its about the data flow before instantiation, not the instantiation itself, and it also avoids confusion with NetworkBehaviour.OnSynchronize()
Let me know if any of these seem closer to NGO’s naming conventions, or if you would prefer a shorter variation
Regarding this, I've tested it, and in the video only one object is an in-scene placed object. The rest are dynamically instantiated through the prefab instance handler, not just scene objects. Maybe I misunderstood what you meant? I’ll work on a simpler example, though to be honest, the linking case is the most valuable use case I’ve found so far, its actually what motivated this feature in the first place. Right now I dont have many alternative examples because most of the benefit comes from enabling that exact flow: having objects pre-created and deterministically selected or connected based on payload metadata. Of course, it also opens the door to more advanced use cases, like sending special setup/configuration data before instantiation (For example in the video that sets up In a way, I don’t yet fully know the limits of the feature, I just know it unlocks workflows that weren’t previously possible. About the other changes, I will answer these and also come with some changes you might like tomorrow. |
I had another pass today. Definitely agree with what you've said about I also took a bit more time with the example. I absolutely see what you're doing there. Thank you for the detailed explanations. It'll be best if the function naming we go with follows the Mixing and matching from your naming options, what do you think of something like these two options? /// Option A
INetworkPrefabInstanceWithDataHandler
OnPreInstantiate()
// Option B
INetworkPrefabWithSynchronizeHandler
OnPrefabSynchronize() |
No problem, I’ll make that change in a few minutes!
Its a perfect idea, making it easier to use for developers. The next commit will include that.
Since it’s still an In contrast, Would this option work for you? public interface INetworkPrefabInstanceHandlerWithData : INetworkPrefabInstanceHandler
{
void OnSynchronizeInstantiationData<T>(ref BufferSerializer<T> serializer) where T : IReadWrite
} The method name Let me know what you think. I’ll go ahead and push a commit with these changes in the meantime and await your feedback. 😄 |
1) Moved the payload deserialization into the AddSceneObject, for deferred instantiation compatibility 2) Changed the new interface to be a direct single extended implementation, instead a complement of the handler 3) Some renames to better match what the feature does for developers
All requested changes have been implemented.
This is the same example shown earlier, but simplified and updated to reflect the new interface and naming conventions: public class TestHandlerDeterministicLink : INetworkPrefabInstanceHandlerWithData
{
Dictionary<int, DeterministicIDHolder> deterministicSpawns = new Dictionary<int, DeterministicIDHolder>();
int nextDeterministicId = 0;
int customSpawnID = 0;
public void OnSynchronizeInstantiationData<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter
{
serializer.SerializeValue(ref customSpawnID);
}
public NetworkObject Instantiate(ulong clientId, Vector3 position, Quaternion rotation)
{
return deterministicSpawns[customSpawnID].GetComponent<NetworkObject>();
}
public void Destroy(NetworkObject networkObject) => GameObject.Destroy(networkObject.gameObject);
public void DoSpawn(GameObject linkablePrefab)
{
var deterministicIdHolder = GameObject.Instantiate(linkablePrefab).GetComponent<DeterministicIDHolder>();
deterministicSpawns[nextDeterministicId] = deterministicIdHolder;
deterministicIdHolder.SetID(nextDeterministicId);
nextDeterministicId++;
}
} Marking the pull request as ready for review ✅ |
Hey @EmandM, marking this as draft again. Feel free to skip the earlier messages. This one reflects the most relevant update.
To solve this cleanly, I'm moving toward a stateless and fully explicit model. For clarity: whenever I refer to "the handler" in this context, I'm specifically talking about This involves:
A typical flow would look like this: networkObject.SetInstantiationData(instantiationData);
//Throws an exception if no handler is associated with compatible data
networkObject.Spawn();
// If data was injected, it will be serialized and used during instantiation.
// If not, an exception is thrown to prevent undefined behavior. Internally, the system can safely check if the injected metadata matches the handler’s expected type and apply it if compatible. And the handler interface would become even simpler: public class MySpawnerWithData : INetworkPrefabInstanceHandlerWithData<T> where T : struct, INetworkSerializable
{
public NetworkObject Instantiate(ulong clientId, Vector3 position, Quaternion rotation, T instantiationData)
{
// Logic based on instantiationData
}
} With this design, the handler is fully stateless and reacts only to the data provided at instantiation. This avoids bugs from reused or outdated values and gives developers precise control over how each object is spawned. It also supports dynamic workflows where an object's context may evolve after creation, without relying on RPCs or mutable state. Since instantiation data is now injected explicitly, the spawning logic no longer needs to handle implicit synchronization. The system stays lean and efficient, with no added overhead and unchanged performance. I’ll finalize the implementation and push the updated version shortly. |
Thanks everyone for all the extra use cases! Makes it easier to reason about what is needed. I think I'm seeing two top level approaches to this type of feature:
or
I was understanding that option 2 was the request, but I see here that option 1 might be more what was intended. Thanks for the patience with this process. Adding new features always take a lot of back and forth and consideration. I've also been sick the last few days, sorry for the delay in responding. |
Yeah, because I initially went with option 2, but real production use quickly exposed issues. The idea is to set metadata once per instance and have it persist across all synchronizations, including for late joiners. Handlers remain fully stateless and act only on the injected data. Hope that clarifies the approach. Really appreciate your input, and hope you're feeling better soon! |
The full implementation is readyHandler definition: public class HandlerWithData : INetworkPrefabInstanceHandlerWithData<MyData>
{
public NetworkObject Instantiate(ulong ownerClientId, Vector3 position, Quaternion rotation, MyData data)
{
//My logic with this instantiationData for instance handling
}
public void Destroy(NetworkObject networkObject) => throw new NotImplementedException();
} Server-side spawn flow: networkObject.InjectInstantiationData(instantiationData);
networkObject.Spawn(); Additionally, I improved the integration tests for this feature and added a new one to simulate late joining. |
Clarifies the purpose and usage of the static data map.
…the ' a caracter (’)
Gentle ping @EmandM, this PR has been ready and production-tested for 3 days. Let me know if there's anything blocking the review or if I can help clarify any part. The implementation includes late join support and updated integration tests. |
Thank you for the updates! Unfortunately we still have some design and refinement to do on this feature. If we are going with option 1, we should consider whether it is necessary to have logic on the prefab handler. If the prefab handler is still a core part of this feature, then we need to keep the logic inside the prefab handler. It's important that as much of the feature as possible lives in one place, that way as NGO grows and shifts over time, this feature is easy to maintain and does not break due to future maintainers not understanding the usage paths of the code. We do not want to expose the intermediate interface to the public (code docs aside, there will be games that build on top of the intermediate interface). As I know a public interface cannot inherit from a private interface, we need to reconsider that approach. I am also uncomfortable with adding a public function to We're super interested in this feature and really want to find a way to get it working. We have a few options we're mulling over internally, though we do not have time to take over the development at this time. |
Okay @EmandM, I’ve moved the injection logic fully inside the prefab handler. This should now fully align with your architectural concerns:
With that in place, the part of the implementation that previously didn’t look ideal now results in two possible approaches: ANetworkManager.Singleton.PrefabHandler.InjectInstantiationData(networkObject, data);
networkObject.Spawn(); It’s now more aligned with the prefab handler conceptually, but relying on a global call still feels somewhat indirect and tightly coupled to the NetworkManager. So, in the latest commit 3dea6aa, I extended the handler interface to support a more intuitive usage: B// data is already type-constrained here, so invalid types cannot be passed
myHandlerWithData.InjectInstantiationData(instance, data);
instance.Spawn();
//or alternatively, just call:
myHandlerWithData.SpawnWithData(instance, data); Once you decide my implementation is solid enough, I’ll mark the PR as ready for review again. You mentioned there were some internal options being considered, I’d be interested in hearing more about them, especially if they could help guide this to completion. Additionally, to be fully honest, having something officially accepted into Unity would mean a lot to me personally. It’s not just about the feature, it would be a meaningful milestone in my career to contribute something that becomes part of the engine. That’s why I’m not giving up. |
I think we're moving in the right direction. Unfortunately we have to slow down the review process a bit for now as we're close enough to a good solution that it takes some time to go over each iteration and work through the side effects and consequences of the changes. Internally we were considering that the api for the I see what you're doing with the If you're willing to hang on through the (longwinded) feature development process, then we're happy to work with you to get this feature landed under your name. It just may take some time as adding new features is a risky process with many future considerations to keep in mind. |
Thanks for the update and the transparency. I understand all your concerns. Regarding the extension methods: About the payload (stateful wrapper) concern: Originally, I avoided modifying the instantiation flow to keep method signatures intact, but I’m now considering a parallel path for payload-based instantiation that preserves existing behavior. Let me know if you're comfortable with me exploring a pure stateless approach to pass the data. I’ll do my best to minimize code changes and maintain full backwards compatibility, though it may take a bit longer than previous iterations. I want to make sure the design aligns well with all the feedback you’ve shared throughout this PR. Also, since I’m already using these changes in real production, I may come back with additional edge cases or refinements as they surface. I’ll make sure any such updates stay aligned with the direction we’re defining here. I really appreciate the opportunity to work on this with you. I’m fully committed to seeing it through, regardless of how long it takes. ❤️ |
Feel free to change any internal methods that need changing. Those are completely free to change at any time! I like the I think it'll work well simply having a branch when the Internally we were considering having a new type of prefab handler that takes a struct as an argument, and that way we have a lot more flexibility with adding features to the prefab (e.g. something like |
I just want to bump a quick comment to this thread that I really like the cooperation here, how this is developing and while I don't have sufficient knowledge to help more with those topics I love the energy in this thread and I'm looking forward to how it develops 👀 (having in mind that it's always complicated process with many bumps on the way) |
I've already implemented the required changes addressing all feedback:
This version aligns closely with the current architecture and feedback. I’d appreciate any thoughts on whether this direction feels more in line with your expectations, especially considering clarity, maintainability, and minimal disruption to existing workflows Additionally, I’ve prepared a branch extending this change with more ambitious improvements for future-proofing Key improvements in that extended branch: Please, check extended branch PR
|
Solves Issue #3421
Related to the discussions in Pull Request #3419 and Issue #3421 (follow-up proposal based on new approach).
This PR introduces support for sending custom instantiation payloads through
INetworkPrefabInstanceHandlerWithData
to receive metadata before callingInstantiate()
.The feature is fully optional, backward-compatible, and requires no changes to existing user workflows.
Changelog
INetworkPrefabInstanceHandlerWithData
, a variant ofINetworkPrefabInstanceHandler
that supports synchronizing custom data prior to theInstantiate()
method call.Testing and Documentation
INetworkPrefabInstanceHandler.Instantiate()
summary updated to mentionINetworkPrefabInstanceHandlerWithData
INetworkPrefabInstanceHandlerWithData
.Deprecated API
Backport
Implementation Example
Spawning flow:
Important
When spawning, you must update the handler's data before calling
Spawn()
orInstantiateAndSpawn()
.The data set in the handler will be serialized automatically prior the instantiation process.
Highlights